Skip to content

JAVA-5774 Allow all property types for Filter.regex() #1619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

jenssuhr
Copy link

While it makes some sense to only allow the regex function on String properties, this also precludes using it with properties that wrap the String in a value class.

To allow for that use case, I think it would make sense to relax the property type and allow anything there.

@rozza
Copy link
Member

rozza commented Feb 4, 2025

I'm not sure I fully understand the usecase could you please add some test cases? That should help.

@jenssuhr jenssuhr force-pushed the MIRA-5774-regex-restrictions branch from 75bd37c to b9f235b Compare February 4, 2025 09:58
@jenssuhr
Copy link
Author

jenssuhr commented Feb 4, 2025

I'm not sure I fully understand the usecase could you please add some test cases? That should help.

Sure, I can do that

@jenssuhr jenssuhr closed this Feb 4, 2025
@jenssuhr jenssuhr deleted the MIRA-5774-regex-restrictions branch February 4, 2025 10:00
@jenssuhr jenssuhr restored the MIRA-5774-regex-restrictions branch February 4, 2025 10:01
@jenssuhr
Copy link
Author

jenssuhr commented Feb 4, 2025

Oops, forgot that renaming the source branch closes the PR

@jenssuhr jenssuhr reopened this Feb 4, 2025
…ter on a non-String (value class) property
@jenssuhr
Copy link
Author

jenssuhr commented Feb 4, 2025

I've added an example for filtering on a property with a value class type, but it would also be useful for data classes with a custom codec that serialize into a string.

@jyemin jyemin added the external label Feb 4, 2025
@rozza
Copy link
Member

rozza commented Feb 12, 2025

@jenssuhr thanks for the clarification.

I'm tempted to say this change maybe too niche for users. With the explicit String type users can rationalize what will happen during encoding. I am concerned that if it is widened out to any type users may not fully understand what will happen during encoding and if they even need a custom codec.

@jenssuhr
Copy link
Author

@rozza Fair point, I think this PR can be closed. If the path() function is made public (see https://jira.mongodb.org/browse/JAVA-5776), it will be easy to create a custom function for this. I agree that it could be confusing if regex is allowed for e.g. an Int property.

@jenssuhr jenssuhr closed this Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants